-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase the maximum unpacked size of a crate to 1GiB (CVE-2022-36114 mitigation) #11151
Conversation
A common use-case of private registries is to host projects that may require slightly higher limits than what is a good fit for the public crates.io registry, such as crates with vendored binary dependencies, or other larger crates that may not fit on crates.io. When supporting many targets (architecture/OS combinations), the size of a crate on a private registry with vendored dependencies is proportional in the number of targets, so as support for more targets is added, a crate can reasonably exceed 512MiB. The limit to pick is somewhat arbitrary, but in the context of CVE-2022-36114 we should be considerate of users with lower-end or older hardware, who may be the most vulnerable to zip-bombs. As a comparison point, on my system building a hello world with `reqwest` that GETs example.org and prints the reply consumes 538M under the target/ folder. Projects beyond hello worlds that include crates from private registries would usually require on the order of 1-10 GiBs free disk space for the target folder. It seems resonable to pick a threshold for treating a Rust crate in an external registry as a zip bomb up to be in the same order of magnitude (or just above) disk usage numbers that a regular user could be expected to encounter in normal, non-malicious scenarios. This commit only bumps the limit to 1GiB based on need, but it may not be entirely unreasonable in the future to consider even slightly higher thresholds to not be zip-bombs, on a "pro re nata" (as needed) basis.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @epage (or someone else) soon. Please see the contribution instructions for more information. |
(cc @joshtriplett @pietroalbini since I think you worked on the previous patch: #11089) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it might deserve a configurable setting to manage this. Just not sure where the best place is for it.
[registries.<name>] # put it here?
max_unpack_size = "?" # in bytes?, accept SI and/or IEC format?
Zulip thread discussion: https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/.2311151.20configurable.20unpack.20size.3F
Would it make sense to use a separate crate for each target? This is what winapi does. The advantage of this is that you only have to download the crates for targets that you actually compile for rather than download the full >512MiB. |
I think splitting by target should be possible in our case, good idea! I also like It's not too uncommon for me to run a cargo command that ends up filling the rest of the drive, as just a regular build can consume a lot of space on my laptop. That said, for some people running out of space unexpectedly can be a serious problem, so the mitigation does something useful and beneficial. So from a cost/benefit point of view, considering false positives, the rarity of the attack scenario in the first place, and the impact of the attack, I think having any sort of option or tunable would be very helpful. I know that for me, as someone who occasionally experiences Another possibility, outside of adding a |
Hello. I am going to close this as it is superseded by #11337. Let's wait for responses from the team. Thank you all for the input so far! |
Hello, we host a crate on a private registry that includes a vendored static C++ library supporting several targets (mobiles and desktop on various architectures). Some releases of the crate have been above 512MB, as the vendored size keeps scaling with the number of supported targets.
With the latest stable cargo, CI is broken for our users due to the zip-bomb detection triggering on previous releases above 512MiB.
The commit message with more rationale for adjusting the zip-bomb threshold follows.
A use-case of private registries is to host projects that may require slightly higher limits than what is a good fit for the public crates.io registry, such as crates with vendored binary dependencies, or other larger crates that may not fit on crates.io.
When supporting many targets (architecture/OS combinations), the size of a crate on a private registry with vendored dependencies is proportional in the number of targets, so as support for more targets is added, a crate can reasonably exceed 512MiB.
The limit to pick is somewhat arbitrary, but in the context of CVE-2022-36114 we should be considerate of users with lower-end or older hardware, who may be the most vulnerable to zip-bombs.
As a comparison point, on my system building a hello world with
reqwest
that GETs example.org and prints the reply consumes 538M under the target/ folder.Projects beyond hello worlds that include crates from private registries would usually require on the order of 1-10 GiBs free disk space for the target folder.
It seems resonable to pick a threshold for treating a Rust crate in an external registry as a zip bomb up to be in the same order of magnitude (or just above) disk usage numbers that a regular user could be expected to encounter in normal, non-malicious scenarios.
This commit only bumps the limit to 1GiB based on need, but it may not be entirely unreasonable in the future to consider even slightly higher thresholds to not be zip-bombs, on a "pro re nata" (as needed) basis.